feat: add local personalized trending feed#4234
Open
quantumvoid0 wants to merge 6 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds personalized trending using watch-history (IndexedDB) with two new preference toggles and translations; fetches per-channel recommendations, deduplicates and interleaves or filters results; plus minor FeedPage null-safety and strict-equality fixes. ChangesPersonalized Trending Feature
FeedPage Code Quality
Sequence Diagram(s)sequenceDiagram
participant User
participant TrendingComponent
participant IndexedDB
participant Network
participant Cache
User->>TrendingComponent: open Trending page
TrendingComponent->>IndexedDB: read watch_history (getPreferredChannels)
TrendingComponent->>Network: fetch base trending
TrendingComponent->>Cache: request per-channel fetch
Cache->>Network: fetch per-channel streams (on cache miss)
Network-->>TrendingComponent: return results
TrendingComponent->>TrendingComponent: dedupe / interleave / filter
TrendingComponent-->>User: render videos
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/components/FeedPage.vue (1)
98-102: 💤 Low valueConsider removing the unnecessary
elseblock.Since the
ifblock on line 97 returns early, theelseis redundant. The code would be cleaner without it:♻️ Optional refactor to remove redundant else
const getRssUrl = computed(() => { if (isAuthenticated()) return authApiUrl() + "/feed/rss?authToken=" + getAuthToken(); - else { - const channels = getUnauthenticatedChannels(); - if (!channels) return null; - return authApiUrl() + "/feed/unauthenticated/rss?channels=" + channels; - } + const channels = getUnauthenticatedChannels(); + if (!channels) return null; + return authApiUrl() + "/feed/unauthenticated/rss?channels=" + channels; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/FeedPage.vue` around lines 98 - 102, The else after the early return is redundant; remove the else block and dedent its body so that when the earlier condition returns, execution continues to call getUnauthenticatedChannels(), check for null, and return authApiUrl() + "/feed/unauthenticated/rss?channels=" + channels; update the branch around getUnauthenticatedChannels() and authApiUrl() in FeedPage.vue (the block containing getUnauthenticatedChannels and the return) accordingly to preserve behavior but eliminate the unnecessary else.src/components/TrendingPage.vue (1)
72-83: 💤 Low valueSimplify
interleavewith a loop instead of three repeated pushes.The repeated
if (ti < trending.length) out.push(trending[ti++]);block makes the 3:1 ratio implicit. A small loop expresses the intent and makes the ratio trivially tunable.♻️ Suggested refactor
function interleave(trending, recommended) { const out = []; let ti = 0, ri = 0; + const trendingPerRecommended = 3; while (ti < trending.length || ri < recommended.length) { - if (ti < trending.length) out.push(trending[ti++]); - if (ti < trending.length) out.push(trending[ti++]); - if (ti < trending.length) out.push(trending[ti++]); + for (let i = 0; i < trendingPerRecommended && ti < trending.length; i++) { + out.push(trending[ti++]); + } if (ri < recommended.length) out.push(recommended[ri++]); } return out; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TrendingPage.vue` around lines 72 - 83, The interleave function currently repeats three identical checks to push trending items; replace those three lines with a small inner loop so the 3:1 ratio is explicit and tunable. In function interleave(trending, recommended) (variables out, ti, ri), inside the while loop run a for-loop that iterates up to 3 times (or until ti >= trending.length) and pushes trending[ti++] each iteration, then keep the single conditional push for recommended[ri++]; this makes the ratio configurable and removes the repeated if statements.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/TrendingPage.vue`:
- Around line 102-113: The dedup logic in dedup currently extracts the video id
via split("v=")[1], which leaves trailing query params and breaks deduping;
change the extraction to parse the v query parameter properly (e.g., use
URL/URLSearchParams or parse the querystring) inside dedup so you get
searchParams.get('v') for each v.url, handle relative URLs by supplying a base
or parsing only the query portion, and keep the same seen Set/return behavior
used by dedup; update references to dedup, seen, interleave,
personalizedTrendingOnly, recommended, and trending accordingly.
- Around line 65-70: fetchChannelVideos currently slices the first 5
relatedStreams without guaranteeing order and re-fetches channel data on every
page load; update fetchChannelVideos to first sort each r.value.relatedStreams
by uploadedDate (newest first) before taking slice(0,5), and implement a
short-lived cache (e.g., store the combined per-channel results keyed by
channelIds in sessionStorage or an in-memory Map with a 5–10 minute TTL) so the
Promise.allSettled call is skipped when cached data is fresh; keep references to
relatedStreams, uploadedDate, and fetchChannelVideos to locate where to add
sorting and cache-check logic.
- Around line 45-63: In getPreferredChannels, guard against empty channel IDs
produced when video.uploaderUrl has trailing slashes by normalizing/extracting
the id and skipping falsy/empty results before incrementing counts; specifically
update the id extraction (from video.uploaderUrl.split("/").pop()) to trim
trailing slashes or otherwise derive a non-empty id and only call counts.set(id,
...) when id is truthy so empty strings can't be counted among preferred
channels.
In `@src/locales/en.json`:
- Around line 136-137: Lines defining the keys "personalized_trending" and
"personalized_trending_only" use tab indentation instead of the project's
4-space JSON indentation; replace any tab characters before those entries with 4
spaces so their indentation matches the rest of src/locales/en.json, then run
the repo formatter (Prettier/EditorConfig) to normalize spacing and remove the
noisy diff.
---
Nitpick comments:
In `@src/components/FeedPage.vue`:
- Around line 98-102: The else after the early return is redundant; remove the
else block and dedent its body so that when the earlier condition returns,
execution continues to call getUnauthenticatedChannels(), check for null, and
return authApiUrl() + "/feed/unauthenticated/rss?channels=" + channels; update
the branch around getUnauthenticatedChannels() and authApiUrl() in FeedPage.vue
(the block containing getUnauthenticatedChannels and the return) accordingly to
preserve behavior but eliminate the unnecessary else.
In `@src/components/TrendingPage.vue`:
- Around line 72-83: The interleave function currently repeats three identical
checks to push trending items; replace those three lines with a small inner loop
so the 3:1 ratio is explicit and tunable. In function interleave(trending,
recommended) (variables out, ti, ri), inside the while loop run a for-loop that
iterates up to 3 times (or until ti >= trending.length) and pushes
trending[ti++] each iteration, then keep the single conditional push for
recommended[ri++]; this makes the ratio configurable and removes the repeated if
statements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3bae360-fe99-4f14-965a-1a2a15276d2f
📒 Files selected for processing (4)
src/components/FeedPage.vuesrc/components/PreferencesPage.vuesrc/components/TrendingPage.vuesrc/locales/en.json
Wasnt able to see this bcz vim had formatted the file cleanly
Author
|
I see there are some issues in the code as reported by coderabbit...ill see to fixing it first |
…e on personalized-only trending
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/TrendingPage.vue (1)
144-174:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent duplicate API requests on first mount when personalizedTrending is enabled.
With the route wrapped in
<keep-alive>(confirmed in App.vue),onActivatedfires immediately afteronMountedon initial load. WhenpersonalizedTrendingis enabled, both lifecycle hooks callfetchTrending(region), resulting in duplicate API requests: 1 extra/trendingcall and up to 10 extra/channel/{id}requests during cold load. TheonActivatedhook also clearschannelCache, forcing all channel data to be refetched unnecessarily.Add a first-mount guard to skip the
onActivatedrefetch on initial mount:♻️ Proposed guard
+const hasMounted = ref(false); + onMounted(() => { if (route.path == import.meta.env.BASE_URL && getPreferenceString("homepage", "trending") == "feed") { return; } const region = getPreferenceString("region", "US"); fetchTrending(region).then(vids => { videos.value = vids; loaded.value = true; updateWatched(videos.value); fetchDeArrowContent(videos.value); + hasMounted.value = true; }); }); onActivated(() => { document.title = t("titles.trending") + " - Piped"; if (videos.value.length > 0) updateWatched(videos.value); if (route.path == import.meta.env.BASE_URL) { const homepage = getHomePage(); if (homepage !== undefined) router.push(homepage); } - if (getPreferenceBoolean("personalizedTrending", false)) { + if (hasMounted.value && getPreferenceBoolean("personalizedTrending", false)) { channelCache.clear();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TrendingPage.vue` around lines 144 - 174, The onMounted and onActivated hooks both call fetchTrending (and clear channelCache) when personalizedTrending is enabled, causing duplicate requests on initial mount; add a first-activation guard: declare a local ref/boolean (e.g., firstActivation = ref(true)) near the component setup, in onActivated check if firstActivation is true then set it to false and skip the personalizedTrending branch (do not clear channelCache or call fetchTrending), and ensure onMounted sets firstActivation to false after its initial work so subsequent activations run the existing personalizedTrending logic; reference symbols: onMounted, onActivated, personalizedTrending, channelCache, fetchTrending, videos, loaded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/TrendingPage.vue`:
- Around line 164-173: The personalized-refetch branch (guarded by
getPreferenceBoolean("personalizedTrending", false)) replaces videos.value but
doesn't call updateWatched, so watched badges aren't reapplied; update the .then
handler on fetchTrending (the block that sets videos.value, toggles
loaded.value, and calls fetchDeArrowContent(videos.value)) to also call
updateWatched(videos.value) after the new videos are set (e.g., call
updateWatched(videos.value) following fetchDeArrowContent) so watched state is
reapplied for the refreshed list.
- Around line 8-12: The empty-state paragraph currently shows whenever
videos.length === 0; change its guard to only render the "no watch history" copy
for the personalized-only path by adding the personalized flag (e.g.
personalizedOnly or personalizedTrending) and ensuring loaded is true before
showing it (so the message is not shown during loading or for non-personalized
empty results). Update the v-if on the paragraph to require videos.length === 0
&& loaded && personalizedOnly (or the appropriate personalizedTrending variable
exported from the <script setup>), and leave a generic/alternative empty-state
message for the non-personalized case.
- Around line 92-95: The sort currently compares the human-readable uploadedDate
strings on r.value.relatedStreams which yields lexicographic ordering; change
the comparator to use the numeric uploaded timestamp (e.g., b.uploaded ?? 0 and
a.uploaded ?? 0) so streams are sorted by most-recent uploaded ms. Locate the
sort call on r.value.relatedStreams, replace the (a, b) => ((b.uploadedDate ??
0) > (a.uploadedDate ?? 0) ? 1 : -1) comparator with one that subtracts the
numeric uploaded values (or otherwise compares b.uploaded and a.uploaded) and
keep the .slice(0, 5) limiting afterwards.
---
Outside diff comments:
In `@src/components/TrendingPage.vue`:
- Around line 144-174: The onMounted and onActivated hooks both call
fetchTrending (and clear channelCache) when personalizedTrending is enabled,
causing duplicate requests on initial mount; add a first-activation guard:
declare a local ref/boolean (e.g., firstActivation = ref(true)) near the
component setup, in onActivated check if firstActivation is true then set it to
false and skip the personalizedTrending branch (do not clear channelCache or
call fetchTrending), and ensure onMounted sets firstActivation to false after
its initial work so subsequent activations run the existing personalizedTrending
logic; reference symbols: onMounted, onActivated, personalizedTrending,
channelCache, fetchTrending, videos, loaded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cfddc4f1-d567-46f0-a691-ebf8b050c9b1
📒 Files selected for processing (2)
src/components/TrendingPage.vuesrc/locales/en.json
✅ Files skipped from review due to trivial changes (1)
- src/locales/en.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/TrendingPage.vue (1)
8-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDerive the empty-state flag from the effective personalized mode.
personalizedOnly.valuecurrently mirrors only the secondary toggle. IfpersonalizedTrendingis off butpersonalizedTrendingOnlyis still stored astrue, an empty plain/trendingresponse will still show the watch-history message.Suggested fix
- personalizedOnly.value = getPreferenceBoolean("personalizedTrendingOnly", false); + personalizedOnly.value = + getPreferenceBoolean("personalizedTrending", false) && + getPreferenceBoolean("personalizedTrendingOnly", false); ... - personalizedOnly.value = getPreferenceBoolean("personalizedTrendingOnly", false); + personalizedOnly.value = + getPreferenceBoolean("personalizedTrending", false) && + getPreferenceBoolean("personalizedTrendingOnly", false);Also applies to: 151-175
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TrendingPage.vue` around lines 8 - 10, personalizedOnly currently mirrors only the secondary toggle and can be true even when personalized trending is disabled; change personalizedOnly to be derived from the effective personalized mode (i.e., require both the main personalizedTrending flag and the secondary personalizedTrendingOnly flag) so it only becomes true when personalizedTrending is enabled AND personalizedTrendingOnly is set; update the places referencing personalizedOnly (the empty-state v-if at the top and the other usage around the 151-175 block) to use this computed/derived value instead of directly mirroring personalizedTrendingOnly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/TrendingPage.vue`:
- Around line 114-144: fetchTrending currently allows failures from
getPreferredChannels() or fetchChannelVideos to bubble up and prevent the
non-personalized trending fallback; wrap the personalized-path async calls (the
Promise.all that invokes getPreferredChannels() and the subsequent
fetchChannelVideos call) in a try/catch so that on any error you fall back to
returning the plain fetchJson(apiUrl() + "/trending", { region: region || "US"
}) result; ensure you still honor personalizedTrendingOnly (i.e., if
personalizedTrendingOnly is true and an error occurs, return the plain trending)
and keep using dedup/interleave as before when the personalized calls succeed.
---
Duplicate comments:
In `@src/components/TrendingPage.vue`:
- Around line 8-10: personalizedOnly currently mirrors only the secondary toggle
and can be true even when personalized trending is disabled; change
personalizedOnly to be derived from the effective personalized mode (i.e.,
require both the main personalizedTrending flag and the secondary
personalizedTrendingOnly flag) so it only becomes true when personalizedTrending
is enabled AND personalizedTrendingOnly is set; update the places referencing
personalizedOnly (the empty-state v-if at the top and the other usage around the
151-175 block) to use this computed/derived value instead of directly mirroring
personalizedTrendingOnly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a4a3a20-df9e-4885-a0ba-d8c944f17f17
📒 Files selected for processing (1)
src/components/TrendingPage.vue
FireMasterK
reviewed
Jun 1, 2026
| const cached = channelCache.get(cacheKey); | ||
| if (cached && Date.now() - cached.ts < CHANNEL_CACHE_TTL) return cached.data; | ||
|
|
||
| const results = await Promise.allSettled(channelIds.map(id => fetchJson(apiUrl() + "/channel/" + id))); |
Member
There was a problem hiding this comment.
Can you use the unauthenticated channel feeds api, since this is quite expensive on the server? You can pass multiple channel IDs to it too.
Author
There was a problem hiding this comment.
surely, ill implement the fix..but there seems to be another issue preventing the code in this PR from running, about a week ago i noticed that videos section in channels no longer load, and this affected the trending page too as it reads from there, i dont think its just my side, bcz the piped.video server also had the same issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR includes a new feature for the piped:trending page which allows users to enable a more personalized trending section from watch history.
It reads watch history from IndexedDB, ranks channels by watch count, fetches recent 5 videos from top 10 channels, Also added getPreferenceBoolean import. This feature can be turned on/off from preferences and also have the ability to show just the personalized section without global trending.
P.S:
Variables that are never reassigned should be const. so changed let->const... No functional difference, just cleaner code.
added RSS guard, without it, the RSS link would render with a URL like /feed/unauthenticated/rss?channels= (empty channels string) when not logged in and no subscriptions exist. Clicking it would hit the API with an empty parameter, likely returning an error or garbage. The null return + v-if just hides the button entirely in that case rather than showing a broken link.
Summary by CodeRabbit
New Features
Bug Fixes